Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Geo variogram #98

Closed
wants to merge 4 commits into from
Closed

Geo variogram #98

wants to merge 4 commits into from

Conversation

LSchueler
Copy link
Member

@LSchueler LSchueler commented Jul 27, 2020 •
edited
Loading

A first step towards supporting spherical coordinate systems #54 is to support variogram estimation on spherical coordinate systems.

This is a first proposal for calculating unstructured variograms on spheres.

A few things to discuss:

  • is spherical a good name for the argument in the function vario_estimate_unstructured?
    • let's use great-circle
  • how are we going to provide the new distance function to users? - This is important in order to, e.g. create bins.
    • Possible options:
      • import the Cython function from estimator.pyx into one of the tools.py namespaces
      • create a new tools.pyx as a more general Cython code collection
      • reimplement the distance function in Python
      • use on of the already existing Python packages as external dependenies, e.g. haversine
    • Suggestion:
      • users shouldn't need to care about the haversine routines (just use it internally)
      • the rest of GSTools should deal with xyz positions converted by a new routine latlon2xyz
      • variogram fitting needs to provide a new workflow to fit associated 3D Yadrenko models to estimated great-circle variograms
      • with all this, we don't need to rewrite the SRF, CovModel and Krige classes
      • we need a set of good tutorials dealing with geo-coordinates (explaining yadrenko models and great-circle distances)
  • better testing: I'm not sure what else to test
  • how could potential pitfalls / confusing arguments be caught (lat-lon, y-x)
    • as written above: conversions routines and good tutorials
  • should we stick to the unit of metre or does kilometre make more sense?
    • we shoud use the great-circle distance in deg and leave this decision to the user

@LSchueler LSchueler requested a review from MuellerSeb July 27, 2020 15:45
@LSchueler LSchueler self-assigned this Jul 27, 2020
@LSchueler LSchueler added the enhancement New feature or request label Jul 27, 2020
@LSchueler LSchueler linked an issue Jul 27, 2020 that may be closed by this pull request
Lat-Lon support (geographical coordinates) #54
Closed
MuellerSeb
MuellerSeb requested changes Aug 18, 2020
View reviewed changes
Copy link
Member

@MuellerSeb MuellerSeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about the geo-coord support recently and I think, it would be the easiest way to provide a converter function latlon2xyz that takes a radius argument. Then we only have to provide a set of good tutorials on how to deal with geo-coord and we don't need to rewrite all the routines.
Only thing we need is a new option in the fit_variogram method or another method called fit_yadrenko to fit a 3D model to the estimated great-circle variogram where a radius can be passed to have a meaningful length scale.
@LSchueler : What do you think?

gstools/variogram/estimator.pyx
pow(sin(diff_lon/2.0), 2)
)
# earth radius for WGS84 ellipsoid r ~ 6371km
return 12742000.0 * atan2(sqrt(arg), sqrt(1.0-arg))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use the great-circle distance in deg as the result, so we don't have to deal with the earth radius. This is also done in Pykrige this way ATM. Then we also don't have to care about km vs m and leave this decision to the user.

gstools/variogram/variogram.py Outdated Show resolved Hide resolved
gstools/variogram/variogram.py Outdated Show resolved Hide resolved
@MuellerSeb MuellerSeb added this to the 1.3 milestone Aug 18, 2020
@MuellerSeb MuellerSeb marked this pull request as draft November 7, 2020 13:34
@MuellerSeb
Copy link
Member

this is now made obsolete by #113

@MuellerSeb MuellerSeb closed this Nov 24, 2020
@MuellerSeb MuellerSeb deleted the geo-variogram branch January 11, 2021 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MuellerSeb MuellerSeb MuellerSeb requested changes

Assignees

@LSchueler LSchueler

Labels
enhancement New feature or request
Projects
None yet
Milestone
1.3
Development

Successfully merging this pull request may close these issues.

Lat-Lon support (geographical coordinates)
2 participants
@LSchueler @MuellerSeb